Feature/tag texts #1345
Feature/tag texts #1345
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1345 +/- ##
===========================================
- Coverage 86.72% 86.71% -0.01%
===========================================
Files 337 337
Lines 84145 84147 +2
Branches 4769 4771 +2
===========================================
- Hits 72971 72968 -3
- Misses 11151 11156 +5
Partials 23 23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I would suggest to always consider tags to be a triple of strings [left, tag, right]. If one does not need the fences, once can leave them empty. This would avoid a all the case analysis for Array and would also simplify types.
Is there a reason why we would not want that?
| * @param {string} tag The tag name (e.g., 1). | ||
| * @param {string} tagId The unique id for that tag (e.g., mjx-eqn:1). | ||
| * @param {string} tagFormat The formatted tag (e.g., "(1)"). | ||
| * @param {string|string[]} tagFormat The formatted tag (e.g., "$\bullet$" or ['(','1',')']). |
There was a problem hiding this comment.
Would it not be easier to always have this as [string, string, string] return type and then use empty strings if no fence is used. That would allow for (left) tags like 1) to be written as ['', '1', ')'].
| return format | ||
| ? `${left}${format}{${tag}}${right}` | ||
| : `${left}${tag}${right}`; | ||
| return format ? [left, `${format}{${tag}}`, right] : [left, tag, right]; |
There was a problem hiding this comment.
How about:
| return format ? [left, `${format}{${tag}}`, right] : [left, tag, right]; | |
| return [left, `format ? ${format}{${tag}} : tag`, right]; |
| 'node', | ||
| 'mrow', | ||
| ParseUtil.internalMath(parser, tag), | ||
| ParseUtil.internalMath(parser, Array.isArray(tag) ? tag.join('') : tag), |
There was a problem hiding this comment.
This could then be simplified to just tag.join('').
| : format.match(/^(\(|\[|\{)(.*)(\}|\]|\))$/)?.slice(1) || [format]; | ||
| const mml = new TexParser( | ||
| '\\text{' + this.currentTag.tagFormat + '}', | ||
| tag.map((part) => `\\text{${part}}`).join(''), |
There was a problem hiding this comment.
This would need to filter out the empty strings.
| const format = this.currentTag.tagFormat; | ||
| const tag = Array.isArray(format) | ||
| ? format | ||
| : format.match(/^(\(|\[|\{)(.*)(\}|\]|\))$/)?.slice(1) || [format]; |
There was a problem hiding this comment.
This could be simplified if we always assume a triple.
This PR changes the handling of tags to make three separate
mtextnodes: one for the left delimiter, one for the tag number, and one for the right delimiter.The
formatTag()andformatRef()functions now take either a string or a triple of strings to use for the tag format. If it is just a string, then parentheses, brackets, and braces are removed from the beginning and ending of the thing to make the triple, otherwise, the string is used as is (and only a singlemtextwill be created).The tests are updated to take these changes into account.